greplogs: refine output for issue triage #11
Conversation
bcmills
force-pushed
the
triage
branch
2 times, most recently
from
7795ea4 to
eaec9ea
Compare
This improves the ergonomics of the command for Go dashboard triage, eliminating the need to set two flags that are essentially always useful.
When --md is set, echo the shell-escaped 'greplogs' command line itself at the beginning of the output to make the command easier to reproduce when pasted into an issue report. Add a --triage flag that adds checkboxes and a summary to the Markdown output. Apply the --omit flag to commits as well as builders, to more easily filter out commits (especially subrepo commits) that are broken across the board.
…ils> tag This avoids some manual editing for pasting long lists of results into GitHub issues.
|
The first three commits look totally fine (and I would be happy to land them but don't see an obvious way to land just part of a PR?). I definitely get where you're going with the fourth commit, but I'm somewhat skeptical of the approach. |
| return out, nil | ||
| } | ||
|
|
||
| _, err = runCommand("go", "mod", "init", "github.com/aclements/go-misc/greplogs/_embed") |
There was a problem hiding this comment.
This was certainly... unexpected. I assume the idea here is that we actually need to get whatever's at HEAD to get the current broken builders and there's not, like, an RPC API we can hit to get this?
If I'm understanding right, this definitely could use a comment explaining the rationale here. (Also, could we instead add a dashboard API to get this?)
The other thing I wonder is how well-defined this is at all. This will get the list of broken builders right now, but greplogs is all about digging back in history. Doesn't this mean it could miss real failures that happened before a builder was marked broken, and surface expected failures after a builder stops being marked broken?
There was a problem hiding this comment.
I assume the idea here is that we actually need to get whatever's at HEAD to get the current broken builders and there's not, like, an RPC API we can hit to get this?
Correct. And even if there was an RPC API, because the configuration is encoded as a Go package we would have to push a new version of some server every time we wanted to update it.
If I'm understanding right, this definitely could use a comment explaining the rationale here.
Done!
Doesn't this mean it could miss real failures that happened before a builder was marked broken, and surface expected failures after a builder stops being marked broken?
Yep! That's why we only use this if the --triage flag is set — we don't have a log of which builders were broken at which commits (and that would be fairly tedious to maintain), so instead we bias toward over-suppressing failures for builders that are currently broken.
(If triage falls too far behind and a builder is already fixed, a user can explicitly notch out additional previously-broken builders using the --omit flag.)
There was a problem hiding this comment.
Correct. And even if there was an RPC API, because the configuration is encoded as a Go package we would have to push a new version of some server every time we wanted to update it.
Don't we have to do redeploy whenever the broken builder list changes anyway? As far as most people are concerned, the broken builders are whatever's grayed out on the dashboard. If the dashboard was also serving this list via an API, it would match up with whatever's grayed out.
(There's a bigger question here of whether it makes sense for this to be configured in a giant Go struct that can't track history and requires a redeploy on any changes, but that's for another time...)
Also omit builders with known issues when --triage is set. For golang/go#52653.
No description provided.